Added wfTransactionalTimeLimit() method and applied it
authorAaron Schulz <aschulz@wikimedia.org>
Sat, 8 Aug 2015 00:08:33 +0000 (17:08 -0700)
committerTim Starling <tstarling@wikimedia.org>
Wed, 12 Aug 2015 22:09:40 +0000 (22:09 +0000)
* Potentially long running POST requests often use multiple transactions,
  talk to multiple services, or defer updates. Try to make sure they have
  a chance to complete all of the work. WMF already sets ignore_user_abort()
  across the board in config, but this applies it to key spots for all
  installs, in addition to bumping the time limit.
* Eventually this can lower the need for high overall time limits.

Bug: T102890
Change-Id: I893ddd773064dcd63b5b24c84c6391974f4b5aee

26 files changed:
RELEASE-NOTES-1.26
includes/DefaultSettings.php
includes/GlobalFunctions.php
includes/actions/Action.php
includes/actions/DeleteAction.php
includes/actions/EditAction.php
includes/actions/RevertAction.php
includes/actions/RollbackAction.php
includes/api/ApiBase.php
includes/api/ApiDelete.php
includes/api/ApiEditPage.php
includes/api/ApiFileRevert.php
includes/api/ApiImageRotate.php
includes/api/ApiImport.php
includes/api/ApiMove.php
includes/api/ApiRevisionDelete.php
includes/api/ApiRollback.php
includes/api/ApiUndelete.php
includes/specialpage/SpecialPage.php
includes/specials/SpecialImport.php
includes/specials/SpecialMergeHistory.php
includes/specials/SpecialMovepage.php
includes/specials/SpecialRevisiondelete.php
includes/specials/SpecialUndelete.php
includes/specials/SpecialUpload.php
includes/specials/SpecialUploadStash.php

index b68c1c9..986ebc3 100644 (file)
@@ -55,6 +55,8 @@ production.
   the store type from $wgObjectCaches. The default is the local database.
 * Interface message overrides in the MediaWiki namespace will now be cached in
   memcached and APC (if available), rather than memcached and local files.
+* $wgTransactionalTimeLimit was added, which controls the request time limit
+  for potentially slow POST requests that need to be as atomic as possible.
 
 ==== External libraries ====
 * Update es5-shim from v4.0.0 to v4.1.5.
index 19d4b89..9fcdf14 100644 (file)
@@ -2078,6 +2078,14 @@ $wgMaxArticleSize = 2048;
  */
 $wgMemoryLimit = "50M";
 
+/**
+ * The minimum amount of time that MediaWiki needs for "slow" write request,
+ * particularly ones with multiple non-atomic writes that *should* be as
+ * transactional as possible; MediaWiki will call set_time_limit() if needed.
+ * @since 1.26
+ */
+$wgTransactionalTimeLimit = 120;
+
 /** @} */ # end performance hacks }
 
 /************************************************************************//**
index 2b9bc25..f2720df 100644 (file)
@@ -3848,9 +3848,9 @@ function wfStripIllegalFilenameChars( $name ) {
 }
 
 /**
- * Set PHP's memory limit to the larger of php.ini or $wgMemoryLimit;
+ * Set PHP's memory limit to the larger of php.ini or $wgMemoryLimit
  *
- * @return int Value the memory limit was set to.
+ * @return int Prior memory limit
  */
 function wfMemoryLimit() {
        global $wgMemoryLimit;
@@ -3874,6 +3874,26 @@ function wfMemoryLimit() {
        return $memlimit;
 }
 
+/**
+ * Set PHP's time limit to the larger of php.ini or $wgTransactionalTimeLimit
+ *
+ * @return int Prior time limit
+ * @since 1.26
+ */
+function wfTransactionalTimeLimit() {
+       global $wgTransactionalTimeLimit;
+
+       $timeLimit = ini_get( 'max_execution_time' );
+       // Note that CLI scripts use 0
+       if ( $timeLimit > 0 && $wgTransactionalTimeLimit > $timeLimit ) {
+               set_time_limit( $wgTransactionalTimeLimit );
+       }
+
+       ignore_user_abort( true ); // ignore client disconnects
+
+       return $timeLimit;
+}
+
 /**
  * Converts shorthand byte notation to integer form
  *
index bb6a4d5..43f03c3 100644 (file)
@@ -407,4 +407,14 @@ abstract class Action {
         * @throws ErrorPageError
         */
        abstract public function show();
+
+       /**
+        * Call wfTransactionalTimeLimit() if this request was POSTed
+        * @since 1.26
+        */
+       protected function useTransactionalTimeLimit() {
+               if ( $this->getRequest()->wasPosted() ) {
+                       wfTransactionalTimeLimit();
+               }
+       }
 }
index be21a6f..841a94d 100644 (file)
@@ -41,6 +41,8 @@ class DeleteAction extends FormlessAction {
        }
 
        public function show() {
+               $this->useTransactionalTimeLimit();
+
                $out = $this->getOutput();
                if ( $this->getContext()->getConfig()->get( 'UseMediaWikiUIEverywhere' ) ) {
                        $out->addModuleStyles( array(
index 6c8440a..eb53f19 100644 (file)
@@ -41,6 +41,8 @@ class EditAction extends FormlessAction {
        }
 
        public function show() {
+               $this->useTransactionalTimeLimit();
+
                if ( $this->getContext()->getConfig()->get( 'UseMediaWikiUIEverywhere' ) ) {
                        $out = $this->getOutput();
                        $out->addModuleStyles( array(
index d025878..c7f3346 100644 (file)
@@ -107,6 +107,8 @@ class RevertAction extends FormAction {
        }
 
        public function onSubmit( $data ) {
+               $this->useTransactionalTimeLimit();
+
                $source = $this->page->getFile()->getArchiveVirtualUrl(
                        $this->getRequest()->getText( 'oldimage' )
                );
index 76d70d7..93669cf 100644 (file)
@@ -36,6 +36,9 @@ class RollbackAction extends FormlessAction {
        }
 
        public function onView() {
+               // TODO: use $this->useTransactionalTimeLimit(); when POST only
+               wfTransactionalTimeLimit();
+
                $details = null;
 
                $request = $this->getRequest();
index 754c0ed..7b71e6c 100644 (file)
@@ -2870,6 +2870,16 @@ abstract class ApiBase extends ContextSource {
                return $this->getResult()->getData();
        }
 
+       /**
+        * Call wfTransactionalTimeLimit() if this request was POSTed
+        * @since 1.26
+        */
+       protected function useTransactionalTimeLimit() {
+               if ( $this->getRequest()->wasPosted() ) {
+                       wfTransactionalTimeLimit();
+               }
+       }
+
        /**@}*/
 }
 
index 6279dfd..bdf02bf 100644 (file)
@@ -39,6 +39,8 @@ class ApiDelete extends ApiBase {
         * result object.
         */
        public function execute() {
+               $this->useTransactionalTimeLimit();
+
                $params = $this->extractRequestParams();
 
                $pageObj = $this->getTitleOrPageId( $params, 'fromdbmaster' );
index b623849..2f1c01c 100644 (file)
@@ -35,6 +35,8 @@
  */
 class ApiEditPage extends ApiBase {
        public function execute() {
+               $this->useTransactionalTimeLimit();
+
                $user = $this->getUser();
                $params = $this->extractRequestParams();
 
index 5517ee0..a49397d 100644 (file)
@@ -38,6 +38,8 @@ class ApiFileRevert extends ApiBase {
        protected $params;
 
        public function execute() {
+               $this->useTransactionalTimeLimit();
+
                $this->params = $this->extractRequestParams();
                // Extract the file and archiveName from the request parameters
                $this->validateParameters();
index c8390b6..7a544ec 100644 (file)
@@ -49,6 +49,8 @@ class ApiImageRotate extends ApiBase {
        }
 
        public function execute() {
+               $this->useTransactionalTimeLimit();
+
                $params = $this->extractRequestParams();
                $rotation = $params['rotation'];
 
index 4154083..735cc7f 100644 (file)
@@ -32,6 +32,8 @@
 class ApiImport extends ApiBase {
 
        public function execute() {
+               $this->useTransactionalTimeLimit();
+
                $user = $this->getUser();
                $params = $this->extractRequestParams();
 
index e42958b..aca4378 100644 (file)
@@ -31,6 +31,8 @@
 class ApiMove extends ApiBase {
 
        public function execute() {
+               $this->useTransactionalTimeLimit();
+
                $user = $this->getUser();
                $params = $this->extractRequestParams();
 
index 2896231..bb5c145 100644 (file)
@@ -32,6 +32,8 @@
 class ApiRevisionDelete extends ApiBase {
 
        public function execute() {
+               $this->useTransactionalTimeLimit();
+
                $params = $this->extractRequestParams();
                $user = $this->getUser();
 
index 02e62a0..6a3346f 100644 (file)
@@ -40,6 +40,8 @@ class ApiRollback extends ApiBase {
        private $mUser = null;
 
        public function execute() {
+               $this->useTransactionalTimeLimit();
+
                $user = $this->getUser();
                $params = $this->extractRequestParams();
 
index 28702b1..cd50ee6 100644 (file)
@@ -30,6 +30,8 @@
 class ApiUndelete extends ApiBase {
 
        public function execute() {
+               $this->useTransactionalTimeLimit();
+
                $params = $this->extractRequestParams();
 
                if ( !$this->getUser()->isAllowed( 'undelete' ) ) {
index eb18b8f..65a4eb9 100644 (file)
@@ -686,4 +686,14 @@ class SpecialPage {
        protected function getGroupName() {
                return 'other';
        }
+
+       /**
+        * Call wfTransactionalTimeLimit() if this request was POSTed
+        * @since 1.26
+        */
+       protected function useTransactionalTimeLimit() {
+               if ( $this->getRequest()->wasPosted() ) {
+                       wfTransactionalTimeLimit();
+               }
+       }
 }
index f9b8ac3..4cdf6dd 100644 (file)
@@ -57,6 +57,8 @@ class SpecialImport extends SpecialPage {
         * @throws ReadOnlyError
         */
        function execute( $par ) {
+               $this->useTransactionalTimeLimit();
+
                $this->setHeaders();
                $this->outputHeader();
 
index 1f0b6d4..7edf961 100644 (file)
@@ -109,6 +109,8 @@ class SpecialMergeHistory extends SpecialPage {
        }
 
        public function execute( $par ) {
+               $this->useTransactionalTimeLimit();
+
                $this->checkPermissions();
                $this->checkReadOnly();
 
index 5682657..e77479f 100644 (file)
@@ -64,6 +64,8 @@ class MovePageForm extends UnlistedSpecialPage {
        }
 
        public function execute( $par ) {
+               $this->useTransactionalTimeLimit();
+
                $this->checkReadOnly();
 
                $this->setHeaders();
index 77ebac3..7c27ac4 100644 (file)
@@ -110,6 +110,8 @@ class SpecialRevisionDelete extends UnlistedSpecialPage {
        }
 
        public function execute( $par ) {
+               $this->useTransactionalTimeLimit();
+
                $this->checkPermissions();
                $this->checkReadOnly();
 
index d7e75bc..14e295c 100644 (file)
@@ -771,6 +771,8 @@ class SpecialUndelete extends SpecialPage {
        }
 
        function execute( $par ) {
+               $this->useTransactionalTimeLimit();
+
                $user = $this->getUser();
 
                $this->setHeaders();
index 6b0bf41..44be81c 100644 (file)
@@ -152,6 +152,8 @@ class SpecialUpload extends SpecialPage {
         * @throws UserBlockedError
         */
        public function execute( $par ) {
+               $this->useTransactionalTimeLimit();
+
                $this->setHeaders();
                $this->outputHeader();
 
index ad329d3..dd90590 100644 (file)
@@ -58,6 +58,8 @@ class SpecialUploadStash extends UnlistedSpecialPage {
         * @return bool Success
         */
        public function execute( $subPage ) {
+               $this->useTransactionalTimeLimit();
+
                $this->stash = RepoGroup::singleton()->getLocalRepo()->getUploadStash( $this->getUser() );
                $this->checkPermissions();